-
Notifications
You must be signed in to change notification settings - Fork 637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
task done #655
base: master
Are you sure you want to change the base?
task done #655
Conversation
cinema/serializers.py
Outdated
model = Order | ||
fields = ("id", "created_at", "tickets") | ||
|
||
def create(self, validated_data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In create/update/delete functions you don't need to return anything
cinema/models.py
Outdated
@@ -84,24 +84,34 @@ class Ticket(models.Model): | |||
row = models.IntegerField() | |||
seat = models.IntegerField() | |||
|
|||
def clean(self): | |||
@staticmethod | |||
def validate_ticket(row, seat, cinema_hall, error): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix annotations in all your functions
cinema/views.py
Outdated
"genres", | ||
"actors" | ||
) | ||
return queryset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To use distinct will be good here
cinema/views.py
Outdated
if movie: | ||
queryset = queryset.filter(movie_id=movie) | ||
|
||
if self.action in "list": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if action is retrieve?
return [int(str_id) for str_id in query_string.split(",")] | ||
|
||
def get_queryset(self): | ||
queryset = self.queryset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call super method instead of accesing this attribute directly
also solve N+1 problem
cinema/views.py
Outdated
@@ -48,11 +79,69 @@ class MovieSessionViewSet(viewsets.ModelViewSet): | |||
queryset = MovieSession.objects.all() | |||
serializer_class = MovieSessionSerializer | |||
|
|||
def get_serializer_class(self): | |||
def get_queryset(self) -> queryset: | |||
queryset = self.queryset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the same
cinema/serializers.py
Outdated
) | ||
|
||
|
||
class MovieSessionDetailSerializer(MovieSessionSerializer): | ||
movie = MovieListSerializer(many=False, read_only=True) | ||
cinema_hall = CinemaHallSerializer(many=False, read_only=True) | ||
taken_places = serializers.SerializerMethodField() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use nested serializer here. HINT: create one more TicketSerializer that will have only row and seat fields.
cinema/views.py
Outdated
return [int(str_id) for str_id in query_string.split(",")] | ||
|
||
def get_queryset(self): | ||
queryset = super().get_queryset() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to use super here - just access it through self.queryset
cinema/views.py
Outdated
def get_queryset(self): | ||
queryset = super().get_queryset() | ||
|
||
actors = self.request.query_params.get("actors") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need this query params only on the list action
cinema/views.py
Outdated
def get_queryset(self) -> queryset: | ||
queryset = super().get_queryset() | ||
|
||
date = self.request.query_params.get("date") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again - you need these query params only on list action
cinema/views.py
Outdated
if self.action in "list": | ||
queryset = ( | ||
queryset | ||
.prefetch_related("movie", "cinema_hall", "tickets") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
movie and cinema_hall needs to be select_ not prefetch_
cinema/views.py
Outdated
if self.action == "list": | ||
return MovieSessionListSerializer | ||
|
||
if self.action == "retrieve": | ||
return MovieSessionDetailSerializer | ||
|
||
return MovieSessionSerializer | ||
|
||
|
||
class OrderSetPagination(PageNumberPagination): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better move pagination to a separate file to better structure your code
cinema/views.py
Outdated
|
||
|
||
class OrderViewSet(viewsets.ModelViewSet): | ||
queryset = Order.objects.all() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is never used because you override queryset in method below
cinema/serializers.py
Outdated
) | ||
|
||
|
||
class TicketRowAndSeatSerializer(serializers.ModelSerializer): | ||
row = serializers.IntegerField() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better like this instead:
class Meta:
model = Ticket
fields = ("row", "seat")
cinema/serializers.py
Outdated
class MovieSessionDetailSerializer(MovieSessionSerializer): | ||
movie = MovieListSerializer(many=False, read_only=True) | ||
cinema_hall = CinemaHallSerializer(many=False, read_only=True) | ||
taken_places = serializers.SerializerMethodField() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so here you need to pass your TicketRowAndSeatSerializer and then you will not need your get_taken_seats method
cinema/views.py
Outdated
if movie: | ||
queryset = queryset.filter(movie_id=movie) | ||
|
||
if self.action in "list": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you check if action equals list twice? can you optimize here?
No description provided.